Skip to content

Thread name#1

Open
Zelnes wants to merge 8 commits intoLbardoux:masterfrom
Zelnes:threadName
Open

Thread name#1
Zelnes wants to merge 8 commits intoLbardoux:masterfrom
Zelnes:threadName

Conversation

@Zelnes
Copy link

@Zelnes Zelnes commented Apr 1, 2017

I've added a feature that binds a thread id to a string.
This allows a user to set a name to several threads.
It gives thins kind of output for this code :

#include "logs.hpp"
#include <thread>

void printNameTest()
{
	mlog::Options::setFormat("[{TYPE} {DATE} {TIME} {THREAD}] ");
	mlog::info("Before binding the TEST value");
	mlog::Options::bindThreadName(std::this_thread::get_id(), "TEST");
	mlog::info("After binding the TEST value");
}

int main(int argc, char const *argv[])
{
	std::thread test(printNameTest);
	test.join();
	return 0;
}

Gives :

user$ g++  test.cpp -o main.out -pthread -std=c++11
user$ ./main.out 
[INFO    04/01/2017 13:42:56 0x7fec67a3e700] Before binding the TEST value
[INFO    04/01/2017 13:42:56 TEST] After binding the TEST value
user$

Assuming we have ./test.cpp and ./logs.hpp

Zelnes added 4 commits April 1, 2017 12:20
…g header, to use a string instead of the thread id. Though, thread id will be use as default value until you reach the directive to add a name to a thread id. You can after change the value, or delete the id-string association
…EAD_NAME, mlog::Options::bindThreadName and mlog::Options::unbindThreadName
* @param[in] name Name that should be displayed instead of an id
* @see mlog::Options::unbidThreadName
*
* @fn void mlog::Options::unbidThreadName(const std::thread::id& id)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A n is missing right here, (unbid instead of unbind), please add it in order to get consistent documentation.

*
* @param[in] id The identifier returned by std::thread::get_id()
* @param[in] name Name that should be displayed instead of an id
* @see mlog::Options::unbidThreadName
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same story here, unbid instead of unbind.

* @var mlog::__details::__Static_declarer::THREAD_NAME
* The container for bounds between thread id and a string value
* @see mlog::Options::bindThreadName
* @see mlog::Options::unbidThreadName
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here again.

logs.hpp Outdated
case -3:
# ifdef MTL_LOG_WITH_THREADS
out << "0x" << std::hex << std::this_thread::get_id() << std::dec;
# ifdef _REENTRANT
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use MTL_LOG_WITH_THREADS macro instead of _REENTRANT.

logs.hpp Outdated
static bool ENABLE_ALPHA_BOOL;
static MTL_LOG_NAMESPACE::__details::__Header FORMAT;

# ifdef _REENTRANT
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MTL_LOG_WITH_THREADS instead of _REENTRANT

logs.hpp Outdated
MTL_LOG_LOCK;
return MTL_LOG_NAMESPACE::Options::FORMAT;
}
# ifdef _REENTRANT
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MTL_LOG_WITH_THREADS

logs.hpp Outdated
{
MTL_LOG_NAMESPACE::Options::THREAD_NAME.insert(std::make_pair(id, name));
}
static void unbidThreadName(const std::thread::id& id)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unbid instead of unbind

logs.hpp Outdated
MTL_LOG_NAMESPACE::Options::C_BLANK,
MTL_LOG_NAMESPACE::Options::isColorEnabled());
MTL_LOG_NAMESPACE::Options::isColorEnabled(),
# ifdef _REENTRANT
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MTL_LOG_WITH_THREADS

* tag will print the thread id
*
* @param[in] id The identifier returned by std::thread::get_id()
* @see mlog::Options::bidThreadName
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bid instead of bind

logs.hpp Outdated
const char *const nocolor, bool colorEnabled)
const char *const nocolor, bool colorEnabled, const void* threads_names)
{
# ifdef _REENTRANT
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MTL_LOG_WITh_THREADS

@Lbardoux
Copy link
Owner

Lbardoux commented Apr 1, 2017

I don't think this is a good idea to provide functions if and only if a special flag or macro was provided.
It causes trouble if this flag or macro suddenly disappears, with undefined symbols.
I do prefer a function that does nothing, easily remove by the compiler.

Also, you could skip the first argument (std::thread::id) by adding it on the function body directly, assuming the function does something only if we're working with threads. Then users won't have to write it by themselve.
In addition, MTL_LOG_LOCK to perform mutual exclusion is a good idea and will save time for me later with python and java wrapping.

Zelnes added 3 commits April 1, 2017 20:16
The functions or now always visible, and they don't need the thread id any more.
If MTL_LOG_WITH_THREADS is defined, then the function will have a job to do, otherwise it returns immediately.
@Zelnes
Copy link
Author

Zelnes commented Apr 1, 2017

I suppose that the fixes added are good, if you mind check one more time, and I think we'll be good !

logs.hpp Outdated
# endif
}
static void unbidThreadName(const std::thread::id& id)
static void unbidThreadName()
Copy link
Owner

@Lbardoux Lbardoux Apr 2, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unbid is still here, and be explicit about functions that doesn't take parameter (consistence with the entire file) with void keyword.

@Lbardoux
Copy link
Owner

Lbardoux commented Apr 2, 2017

It looks well, but one more complaint, why void* ?
I mean you could easily use a typedef on __details scope, and then use a pointer to that specific type. It also avoid this reinterpret cast.

@Lbardoux
Copy link
Owner

Lbardoux commented Apr 2, 2017

Tested with openMP with a simple pragma omp parallel and it seems to work

…an be changed when we find a way to use thread with mingw.Addition of MTL_THREAD_ID that is thread id type when MTL_LOG_WITH_THREADS is defined, otherwise it's int type. This macro avoid errors. The display function contains the THREAD parameter only if MTL_LOG_WITH_THREADS is defined. Modification of bindThreadName, now there are two functions. The first one binds the given name to the current thread id, while the other one binds a specific thread id to the given name.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants